Skip to content

fix: add GraphQL cache invalidation for programs and modules#3387

Merged
arkid15r merged 13 commits intoOWASP:mainfrom
HarshitVerma109:fix/mentorship-cache-invalidation
Jan 24, 2026
Merged

fix: add GraphQL cache invalidation for programs and modules#3387
arkid15r merged 13 commits intoOWASP:mainfrom
HarshitVerma109:fix/mentorship-cache-invalidation

Conversation

@HarshitVerma109
Copy link
Contributor

Proposed change

Resolves #3348

This PR fixes the mentorship portal cache invalidation issue where updates to programs and modules were not visible immediately after saving. The backend GraphQL resolver cache was not being cleared after mutations, causing stale data to be served.

Backend Changes:

  • Created graphql_cache.py utility to invalidate specific cache entries
  • Added cache invalidation to update_program and update_program_status mutations
  • Added cache invalidation to create_module and update_module mutations

Frontend Changes:

  • Added refetchQueries + awaitRefetchQueries: true to mutation calls
  • Added fetchPolicy: 'cache-and-network' to detail pages

Checklist

  • Required: I followed the contributing workflow
  • Required: I verified that my code works as intended and resolves the issue as described
  • Required: I ran make check-test locally: all warnings addressed, tests passed
  • I used AI for code, documentation, tests, or communication related to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 17, 2026

Summary by CodeRabbit

  • Performance Improvements

    • Added intelligent caching for program and module queries with network fallback strategy.
  • Bug Fixes

    • Corrected loading spinner display logic to show only when data is unavailable.
  • Improvements

    • Program and module lists automatically refresh after create and update operations.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Adds a GraphQL resolver caching extension and top-level cache helpers, wires cache invalidation into program/module mutations via transaction.on_commit, updates frontend queries to use cache-and-network, and changes mutations to await refetchQueries to ensure fresh data is shown after writes.

Changes

Cohort / File(s) Summary
Backend: Cache extension module
backend/apps/api/internal/extensions/cache.py, backend/apps/api/internal/extensions/__init__.py
New CacheExtension implementation plus helpers: generate_key, invalidate_cache, invalidate_program_cache, invalidate_module_cache, and protected-field detection.
Backend: Common extensions refactor
backend/apps/common/extensions.py
Moved key-generation logic out of class to top-level generate_key() and added invalidate helper wrappers to align with new internal extensions module.
Backend: Mutation cache invalidation
backend/apps/mentorship/api/internal/mutations/program.py, backend/apps/mentorship/api/internal/mutations/module.py
Schedule cache invalidation via transaction.on_commit() for create/update flows; capture old keys and invalidate old/new keys when module/program keys change.
Backend: GraphQL settings & tests
backend/settings/graphql.py, backend/tests/apps/api/internal/extensions/cache_test.py
Updated import path for CacheExtension; tests rewritten to use the new module-level helpers and added tests for invalidation helpers and key generation.
Frontend: Query fetch policy & loading behavior
frontend/src/app/mentorship/.../page.tsx, frontend/src/app/my/mentorship/.../page.tsx, frontend/src/app/.../modules/.../page.tsx
Added fetchPolicy: 'cache-and-network' to detail queries and changed loading guard to show spinner only when isLoading && !data. (Multiple files under frontend/src/app/... updated.)
Frontend: Mutations await refetch
frontend/src/app/my/mentorship/programs/create/page.tsx, frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx, frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx, frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx
Mutations now include refetchQueries and awaitRefetchQueries: true; replaced manual cache writes with explicit refetch in module creation.
Frontend: Tests
frontend/__tests__/unit/pages/CreateProgram.test.tsx
Relaxed mutation call assertion to expect.objectContaining(...) to allow added fields (awaitRefetchQueries, refetchQueries).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding GraphQL cache invalidation for programs and modules, which directly addresses the stale data issue.
Description check ✅ Passed The description is well-related to the changeset, clearly explaining the problem, backend changes, frontend changes, and references the linked issue.
Linked Issues check ✅ Passed All three main objectives from issue #3348 are successfully implemented: backend cache invalidation utility [#3348], awaitRefetchQueries in mutations [#3348], and cache-and-network fetch policy on detail pages [#3348].
Out of Scope Changes check ✅ Passed All code changes directly align with the linked issue requirements; no out-of-scope modifications detected. Changes focus exclusively on cache invalidation and fetch policies.
Docstring Coverage ✅ Passed Docstring coverage is 95.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (1)

28-30: Missing refetch or state update after status mutation.

The mutation doesn't include refetchQueries to refresh the data after a successful status update. While the toast indicates success, the displayed status on the page won't update because program remains stale in local state.

Per the PR objectives, mutations should use refetchQueries with awaitRefetchQueries: true to ensure fresh data is fetched after writes.

Suggested fix
   const [updateProgram] = useMutation(UpdateProgramStatusDocument, {
     onError: handleAppError,
+    refetchQueries: [{ query: GetProgramAndModulesDocument, variables: { programKey } }],
+    awaitRefetchQueries: true,
   })

Alternatively, you could optimistically update the local state in the updateStatus function after a successful mutation:

       await updateProgram({
         variables: {
           inputData: {
             key: program.key,
             name: program.name,
             status: newStatus,
           },
         },
       })
+
+      setProgram((prev) => (prev ? { ...prev, status: newStatus } : null))
frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx (1)

116-124: Add type guard or optional chaining for error handling.

err in catch blocks has type unknown. Direct property access without a type guard could fail at runtime if the error isn't an Error object. The create program page uses safer optional chaining (err?.message).

Suggested fix
     } catch (err) {
       addToast({
         title: 'Creation Failed',
-        description: err.message || 'Something went wrong while creating the module.',
+        description: err instanceof Error ? err.message : 'Something went wrong while creating the module.',
         color: 'danger',
         variant: 'solid',
         timeout: 4000,
       })
     }
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (1)

35-53: Error handling may misclassify network errors as "not found".

When error is present but not a "not found" error (e.g., network failure) and module is null (no cached data), the UI shows "Module Not Found" (404) which is misleading. Consider distinguishing between actual 404s and other errors without cached data.

💡 Suggested improvement
   if (isLoading) return <LoadingSpinner />

+  if (error && !error.message?.toLowerCase().includes('not found')) {
+    return (
+      <ErrorDisplay
+        statusCode={500}
+        title="Error Loading Module"
+        message="An error occurred while loading the module. Please try again."
+      />
+    )
+  }
+
   if (!module) {
     return (
       <ErrorDisplay
         statusCode={404}
         title="Module Not Found"
         message="Sorry, the module you're looking for doesn't exist."
       />
     )
   }
🤖 Fix all issues with AI agents
In `@frontend/__tests__/unit/pages/CreateProgram.test.tsx`:
- Around line 162-178: The test currently only asserts awaitRefetchQueries on
mockCreateProgram and misses asserting refetchQueries; update the assertion for
mockCreateProgram (the expect(mockCreateProgram).toHaveBeenCalledWith(...)
block) to also include a refetchQueries property with the expected query (or at
least an array) so the mutation call contains both awaitRefetchQueries: true and
refetchQueries: [/* expected query or queries */]; adjust the
expect.objectContaining to include refetchQueries alongside variables and
awaitRefetchQueries to lock in cache-refresh behavior.

In `@frontend/src/app/mentorship/programs/`[programKey]/page.tsx:
- Around line 20-24: The current useQuery call (GetProgramAndModulesDocument
with variables: { programKey } and fetchPolicy: 'cache-and-network')
unconditionally shows a spinner when loading is true, which hides cached data;
update the render logic that checks loading (the spinner at the current page
component) to only show the spinner when loading is true AND data is falsy
(e.g., if (!data && loading) show spinner), so cached data renders while the
network request proceeds; apply the same change to the analogous modules page
component that uses useQuery there.
- Around line 30-34: Extract the 404 detection into a reusable helper named
isNotFoundError(error) (place it in a shared module like utils/errorUtils or
lib/graphql/errorUtils) and replace the inline checks that use
graphQLRequestError.message?.toLowerCase().includes('not found') in page.tsx and
modules/[moduleKey]/page.tsx with if (isNotFoundError(graphQLRequestError)) {
... } else { handleAppError(graphQLRequestError) }; implement isNotFoundError to
first inspect error.graphQLErrors[].extensions.code for 'NOT_FOUND' or
'RESOURCE_NOT_FOUND', then check error.networkError?.statusCode/response status
for 404, and finally fall back to a case-insensitive message substring match to
preserve current behavior. Ensure the helper is exported and imported where used
so both files share the same logic.
🧹 Nitpick comments (4)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (1)

32-37: Consider refining the loading state to avoid UI flicker during background refetch.

The fetchPolicy: 'cache-and-network' addition aligns well with the PR's cache invalidation objectives. However, combined with notifyOnNetworkStatusChange: true, the loading boolean will be true during background refetches, which could cause the LoadingSpinner to briefly replace existing content.

Consider checking for cached data presence or using networkStatus to distinguish initial load from background refetch:

Suggested improvement
-  const { data, loading: isQueryLoading } = useQuery(GetProgramAndModulesDocument, {
+  const { data, loading: isQueryLoading, networkStatus } = useQuery(GetProgramAndModulesDocument, {
     variables: { programKey },
     skip: !programKey,
     fetchPolicy: 'cache-and-network',
     notifyOnNetworkStatusChange: true,
   })
 
-  const isLoading = isQueryLoading
+  // Only show loading spinner on initial load, not background refetch
+  const isLoading = isQueryLoading && !data

This ensures the spinner only appears when there's no cached data to display, preventing flicker during background network requests.

frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (1)

32-39: Consider extracting duplicate isNotFound computation.

The same error.message?.toLowerCase().includes('not found') logic is computed both in the useEffect (line 34) and in the render block (line 44). While the overhead is negligible, extracting this to a single derived value improves maintainability.

♻️ Suggested refactor
+ const isNotFound = error?.message?.toLowerCase().includes('not found')
+
  useEffect(() => {
    if (error) {
-     const isNotFound = error.message?.toLowerCase().includes('not found')
      if (!isNotFound) {
        handleAppError(error)
      }
    }
- }, [error])
+ }, [error, isNotFound])

  if (isLoading) return <LoadingSpinner />

  if (error) {
-   const isNotFound = error.message?.toLowerCase().includes('not found')
    return (
      <ErrorDisplay
        statusCode={isNotFound ? 404 : 500}

Also applies to: 43-56

backend/apps/common/graphql_cache.py (2)

10-10: Consider adding debug logging for cache invalidation.

The logger is imported but unused. Adding debug-level logging in invalidate_graphql_cache would help diagnose cache issues in production.

♻️ Optional: Add debug logging
 def invalidate_graphql_cache(field_name: str, field_args: dict) -> bool:
     ...
     key = f"{field_name}:{json.dumps(field_args, sort_keys=True)}"
     cache_key = f"{settings.GRAPHQL_RESOLVER_CACHE_PREFIX}-{hashlib.sha256(key.encode()).hexdigest()}"
-    result = cache.delete(cache_key)
-    return result
+    deleted = cache.delete(cache_key)
+    logger.debug("Cache invalidation for %s: %s", field_name, "hit" if deleted else "miss")
+    return deleted

25-26: Minor: Simplify return statement.

Per static analysis (RET504), the intermediate assignment is unnecessary.

♻️ Suggested simplification
-    result = cache.delete(cache_key)
-    return result
+    return cache.delete(cache_key)

Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CR's comments are unaddressed.

@arkid15r arkid15r marked this pull request as draft January 17, 2026 23:13
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (1)

93-114: Potential null reference error due to state sync timing.

With cache-and-network and notifyOnNetworkStatusChange: true, when cached data exists:

  1. isLoading = true (background fetch), data = cached value, but program = null (useEffect hasn't run)
  2. Line 93: true && !cachedValue → false (continues)
  3. Line 95: !null && !true → false (continues)
  4. Line 106: program.status → TypeError

Unlike ModuleDetailsPage which derives programModule directly from data, this component stores it in state via useEffect, creating a timing gap.

🔧 Suggested fix: Guard against null program before accessing properties
-  if (isLoading && !data) return <LoadingSpinner />
+  if (isLoading && !program) return <LoadingSpinner />

   if (!program && !isLoading) {

Alternatively, derive program directly from data like the other page does:

-  const [program, setProgram] = useState<Program | null>(null)
-  const [modules, setModules] = useState<Module[]>([])
...
-  useEffect(() => {
-    if (data?.getProgram) {
-      setProgram(data.getProgram)
-      setModules(data.getProgramModules || [])
-    }
-  }, [data])
+  const program = data?.getProgram ?? null
+  const modules = data?.getProgramModules ?? []

This eliminates the race condition by deriving state synchronously from data.

@HarshitVerma109 HarshitVerma109 marked this pull request as ready for review January 18, 2026 02:52
coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 18, 2026
Copy link
Collaborator

@rudransh-shrivastava rudransh-shrivastava left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, I tested this and the invalidation works.

However, the frontend throws an error after I visit a program.
Error:

Cannot read properties of null (reading 'status') in src/app/my/mentorship/programs/[programKey]/page.tsx (106:53) @ ProgramDetailsPage

Refreshing the page after this, loads the program. I was not able to reproduce it consistently, but this happens when I do the following:

  1. Visit any single program page
  2. Use breadcrumb "Mentorship" to go back to programs list page
  3. Visit any single program page (Errors)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@backend/apps/common/extensions.py`:
- Around line 68-87: invalidate_program_cache and invalidate_module_cache
currently build cache key dicts using camelCase names (programKey, moduleKey)
which don't match the resolver kwargs produced by Strawberry (snake_case), so
cache invalidation never hits; update both methods to pass snake_case keys
(e.g., {"program_key": program_key} and {"module_key": module_key,
"program_key": program_key}) when calling
self.invalidate_cache("getProgram"/"getModule") so the generated keys match the
middleware's resolver kwargs.
🧹 Nitpick comments (1)
backend/apps/common/extensions.py (1)

105-106: Verify SchemaExtension can be instantiated without execution_context.

_cache_extension = CacheExtension() is created at import time. In Strawberry, SchemaExtension often expects an execution_context in __init__. If that’s required in v0.289.0, this will raise at startup. Consider refactoring invalidation helpers to avoid constructing SchemaExtension directly (e.g., extract a standalone key helper), or confirm the constructor accepts None.

@HarshitVerma109
Copy link
Contributor Author

Hey @rudransh-shrivastava, please review this PR. It seems fine to me, but CodeRabbit is showing an error. Could you help me check if any changes are required?

coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 22, 2026
Copy link
Collaborator

@rudransh-shrivastava rudransh-shrivastava left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, looks good to me!

Please add relevant backend tests as per CONTRIBUTING.md

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@backend/apps/mentorship/api/internal/mutations/module.py`:
- Around line 123-124: The cache invalidation call to invalidate_program_cache
is being invoked inside a transaction.atomic block; change this so invalidation
runs only after a successful commit by registering it with transaction.on_commit
(e.g., replace direct invalidate_program_cache(program.key) with
transaction.on_commit(lambda: invalidate_program_cache(program.key))) or move
the call outside the atomic block, ensuring you reference the same
invalidate_program_cache and the surrounding transaction.atomic context so
invalidation only occurs post-commit.
♻️ Duplicate comments (1)
backend/apps/mentorship/api/internal/mutations/module.py (1)

407-409: Same on-commit concern applies here.
Consider batching invalidation inside a single transaction.on_commit callback to prevent stale recache windows.

🔧 Suggested fix
-        invalidate_module_cache(old_module_key, module.program.key)
-        if module.key != old_module_key:
-            invalidate_module_cache(module.key, module.program.key)
+        def _invalidate():
+            invalidate_module_cache(old_module_key, module.program.key)
+            if module.key != old_module_key:
+                invalidate_module_cache(module.key, module.program.key)
+        transaction.on_commit(_invalidate)

coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 22, 2026
@HarshitVerma109
Copy link
Contributor Author

Hey @rudransh-shrivastava,
please review it and let me know if any changes are required

@rudransh-shrivastava
Copy link
Collaborator

@HarshitVerma109 I have already approved it. Also, you need to add the backend tests as I have mentioned in my previous review.

coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 23, 2026
@HarshitVerma109
Copy link
Contributor Author

Hey @rudransh-shrivastava, I just added a backend test. Please take a look and let me know if any changes are needed!

kasya
kasya previously approved these changes Jan 24, 2026
Copy link
Collaborator

@kasya kasya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HarshitVerma109 That's really cool! Thank you!

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@backend/apps/api/internal/extensions/cache.py`:
- Around line 37-49: Update generate_key to use a defensive JSON encoder so
non-JSON-native types (e.g., datetime, UUID, Decimal) serialize correctly:
replace json.dumps(field_args, sort_keys=True) with json.dumps(field_args,
sort_keys=True, cls=DjangoJSONEncoder) and import DjangoJSONEncoder from
django.core.serializers.json; also add a unit test in TestGenerateKey
(backend/tests/apps/api/internal/extensions/cache_test.py) that passes args
containing a datetime and a UUID to generate_key to assert it returns a stable,
non-erroring string that includes settings.GRAPHQL_RESOLVER_CACHE_PREFIX.

@arkid15r arkid15r enabled auto-merge January 24, 2026 03:54
@sonarqubecloud
Copy link

@codecov
Copy link

codecov bot commented Jan 24, 2026

Codecov Report

❌ Patch coverage is 97.67442% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.61%. Comparing base (fd10a28) to head (83bb75e).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
.../my/mentorship/programs/[programKey]/edit/page.tsx 50.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3387      +/-   ##
==========================================
+ Coverage   85.56%   85.61%   +0.04%     
==========================================
  Files         461      461              
  Lines       14258    14254       -4     
  Branches     1907     1905       -2     
==========================================
+ Hits        12200    12203       +3     
+ Misses       1679     1672       -7     
  Partials      379      379              
Flag Coverage Δ
backend 84.48% <100.00%> (+0.01%) ⬆️
frontend 88.73% <92.85%> (+0.15%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
backend/apps/api/internal/extensions/cache.py 100.00% <100.00%> (ø)
...programs/[programKey]/modules/[moduleKey]/page.tsx 100.00% <100.00%> (ø)
.../src/app/mentorship/programs/[programKey]/page.tsx 86.95% <100.00%> (ø)
...ams/[programKey]/modules/[moduleKey]/edit/page.tsx 76.47% <100.00%> (+0.35%) ⬆️
...programs/[programKey]/modules/[moduleKey]/page.tsx 100.00% <100.00%> (ø)
...ship/programs/[programKey]/modules/create/page.tsx 69.23% <100.00%> (+6.93%) ⬆️
...c/app/my/mentorship/programs/[programKey]/page.tsx 90.00% <100.00%> (-0.91%) ⬇️
...end/src/app/my/mentorship/programs/create/page.tsx 97.14% <100.00%> (+0.08%) ⬆️
.../my/mentorship/programs/[programKey]/edit/page.tsx 67.74% <50.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd10a28...83bb75e. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nicely done 👍

@arkid15r arkid15r added this pull request to the merge queue Jan 24, 2026
Merged via the queue into OWASP:main with commit 4643156 Jan 24, 2026
36 of 37 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Jan 31, 2026
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix Stale Data Persistence in Mentorship Portal (Cache Invalidation)

4 participants

Comments